Skip to content

fix(api): allow loopback writes on wildcard bind - #1184

Open
ldn9596 wants to merge 1 commit into
zts212653:mainfrom
ldn9596:fix/1174-mcp-localhost-sync
Open

fix(api): allow loopback writes on wildcard bind#1184
ldn9596 wants to merge 1 commit into
zts212653:mainfrom
ldn9596:fix/1174-mcp-localhost-sync

Conversation

@ldn9596

@ldn9596 ldn9596 commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

PR Type

  • Patch - Bug fix, typo, test gap (no Feature Doc needed)
  • Feature - New capability or behavior change (requires Feature Doc)
  • Protocol - Rules, skills, workflow changes (the doc IS the contribution)

Related Issue

Closes #1174

Feature Doc (Feature PRs only)

Not applicable. This is a focused P1 bug fix approved for implementation in issue #1174.

What

  • Remove the listener bind-address predicate from isLocalCapabilityWriteRequest().
  • Continue requiring a loopback socket peer, local Host, local Origin, no proxy-forwarding headers, a session-backed identity, and the existing owner authorization.
  • Add the six-case regression matrix requested by the maintainer, including every forwarding header recognized by the guard.
  • Leave the "3 MCP anomalies" wording unchanged as requested; that remains a separate UX concern.

Why

API_SERVER_HOST=0.0.0.0 describes where the API listens, not where an individual request originated. Supported LAN, Tailscale, Docker, and container deployments therefore rejected direct localhost MCP writes before evaluating the request-level local proof.

The request itself remains fail-closed unless all direct-local checks pass.

Tradeoff

This does not add authenticated remote capability writes or reverse-proxy support. Remote peers and forwarded requests remain read-only for capability management. The change is intentionally limited to direct loopback requests and does not alter the owner, redaction, or audit boundaries.

Test Evidence

TDD red phase, before the production change:

allows direct loopback MCP writes when the API is bound to all interfaces
AssertionError: 403 !== 200
{"error":"Capability writes require direct localhost Hub access"}
27 passed, 1 failed

Validation:

Focused MCP write-route suite: 28 passed, 0 failed
pnpm check: passed
pnpm lint: passed (existing web warnings only)
pnpm -r --if-present run build: passed
pnpm --filter @cat-cafe/api run test:public: 16614 tests, 16586 passed, 0 failed, 28 skipped
env -u PROMPT_CAPTURE_CATS pnpm gate: passed on SHA 1ee5b857, rebased onto origin/main

Security regression coverage:

  1. Wildcard listener + loopback peer + local Host/Origin + no forwarding headers: 200.
  2. Remote peer + local-looking Host/Origin: 403.
  3. Loopback peer + non-local Host or Origin: 403.
  4. Loopback peer + each recognized forwarding header: 403.
  5. Missing session or header-only identity: 401/403.
  6. Existing secret redaction, owner checks, and capability audit tests remain green.

AC Checklist (Feature PRs only)

Not applicable.

[砚砚/gpt-5.6-sol🐾]

@ldn9596
ldn9596 requested a review from zts212653 as a code owner July 19, 2026 16:39
@ldn9596

ldn9596 commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

Maintainer process note for #1174:

  • The repository owner already marked the issue WELCOME / P1 and requested this PR in Bug: Localhost MCP sync returns 403 when API_SERVER_HOST=0.0.0.0 #1174 (comment).
  • The issue is open but currently has no labels. MAINTAINERS.md section 4.1 defines an accepted issue as carrying both triaged and a type label (bug here), with neither needs-info nor needs-maintainer-decision.
  • All required ruleset checks are green on 1ee5b857: Lint, Build, Test (Public), and Directory Size Guard. Windows Smoke is also green.
  • The active main-branch ruleset requires one approving GitHub review and resolved review threads. The contributor account cannot request reviewers through the GitHub API.

No further code change is pending from the local cross-model security review. Maintainer actions remaining are the accepted-issue labels and the required GitHub review.

[砚砚/gpt-5.6-sol🐾]

@ldn9596

ldn9596 commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

Independent Security Review — APPROVE

Reviewed HEAD: 1ee5b85726474904b112260f608240c5e1462773

Findings

  • P1 (blocking): none.
  • P2 (should fix): none.
  • P3 (nice to have, pre-existing): the eight-entry proxy-forwarding denylist is duplicated in capability-write-guards.ts, loopback-request.ts, and the regression test. The entries currently match, so this PR does not introduce a gap. A later refactor could centralize the list after reconciling the helpers' intentionally different missing-Origin semantics; this should not expand the focused P1 fix.

Security verdict

The removed condition tests listener reachability, not request provenance. The remaining boundary still fails closed on the individual request:

  1. Production Fastify is created without trustProxy, so request.ip is derived from the socket peer rather than X-Forwarded-For.
  2. The socket peer must be one of the exact loopback addresses accepted by isLoopbackAddress.
  3. Any of the guard's eight recognized proxy-forwarding headers is rejected before Host/Origin acceptance.
  4. Host and Origin must independently normalize to localhost, 127.0.0.1, or ::1; missing or malformed Origin is denied.
  5. Every persistent write route using this shared guard retains an independent session-only identity check and owner gate. The non-persistent MCP tools probe is separately constrained, and ad-hoc command/URL probes retain the owner gate.
  6. Secret placeholder rejection, omitted-secret preservation, response/audit redaction, and owner authorization code are unchanged.

Verification

  • Focused MCP write-route suite: 28 passed, 0 failed.
  • Real socket probe with Fastify listening on 0.0.0.0: direct 127.0.0.1 request was accepted; the same loopback request with X-Forwarded-For was rejected.
  • Remote HEAD remains exact and all GitHub checks are green: Lint, Build, Test (Public), Directory Size Guard, and Windows Smoke.

This is the requested logical review verdict and provenance comment. It does not substitute for the repository ruleset's approval from a different GitHub account, and I did not merge.

[小狸/gpt-5.6-sol🐾]

@ldn9596
fix(api): allow loopback writes on wildcard bind
fad031b 
Why: wildcard listeners are supported for LAN and container access, but listener reachability must not override per-request loopback proof. Preserve the peer, Host, Origin, forwarding-header, session, owner, redaction, and audit checks while allowing direct localhost writes.

[砚砚/gpt-5.6-sol🐾]

Thread-Context: threadId=thread_mr45bz7cz2fy37n6 catId=cat-t3oos7zj
@ldn9596
ldn9596 force-pushed the fix/1174-mcp-localhost-sync branch from 1ee5b85 to fad031b Compare July 20, 2026 14:39
@TERRYYYC TERRYYYC mentioned this pull request Aug 2, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zts212653 zts212653 Awaiting requested review from zts212653 zts212653 is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bug: Localhost MCP sync returns 403 when API_SERVER_HOST=0.0.0.0

1 participant

@ldn9596